Skip to content

doc(store): remove outdated instruction for aurelia-cli - #409

Merged
EisenbergEffect merged 1 commit into
aurelia:masterfrom
3cp:update-store-doc
May 24, 2019
Merged

doc(store): remove outdated instruction for aurelia-cli#409
EisenbergEffect merged 1 commit into
aurelia:masterfrom
3cp:update-store-doc

Conversation

@3cp

@3cp 3cp commented May 22, 2019

Copy link
Copy Markdown
Member

@zewa666 can you have a look? I remove the mention of rxjs-compat because the rxjs migration guide covered that, but I have no idea how to use rxjs-compat even after reading the migration guide.

The guide mentioned npm install rxjs@6 rxjs-compat@6 --save but has no code to import 'rxjs-compat';. How does it work?

@3cp

3cp commented May 22, 2019

Copy link
Copy Markdown
Member Author

@EisenbergEffect @zewa666 while reading aurelia-store doc, I noticed the missing object.entries polyfill from aurelia-polyfills.

Since aurelia-store is an official offering from Aurelia, I think aurelia-polyfills should cover that polyfill to smooth user experience. In fact, that polyfill is tiny, about 10 lines. Let me know what you think, I can follow up with PRs to aurelia-polyfills and store documentation.

@EisenbergEffect

Copy link
Copy Markdown
Contributor

@3cp I seem to have some trace of a memory about why we didn't include that polyfill. However, if @zewa666 thinks it would be a good idea, I'm game for it.

@zewa666

zewa666 commented May 24, 2019

Copy link
Copy Markdown
Member

@EisenbergEffect here is the related PR for the polyfill with some discussion. aurelia/polyfills#64 I've talked to @3cp and we'd favour to get it in

@zewa666

zewa666 commented May 24, 2019

Copy link
Copy Markdown
Member

PR looks good, the compat approach is no longer needed. Thanks @3cp for the fix. @EisenbergEffect ready for merge

@EisenbergEffect
EisenbergEffect merged commit cffa7f4 into aurelia:master May 24, 2019
@3cp
3cp deleted the update-store-doc branch May 29, 2019 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants